feat!: XDG directory layout via VpDirs - #2346
Conversation
✅ Deploy Preview for viteplus-preview canceled.
|
b8069a5 to
2f8be03
Compare
08b7589 to
4e2abdf
Compare
|
@liangmiQwQ Your refactor needs to wait until this merge is completed first, as the changes are expected to be quite significant. |
78a23b6 to
a433ed4
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
6b26e69 to
ca9dcca
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca9dcca689
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
IMO, I would be a little bit curious about the benefit for this move. The current For example, I help maintain the Vite+'s global package installations in several, I'm concerned that using I'm not against this direction, I just want to understand the motivation and the problem it solves. If it does help Vite+ improve, then now is really a good time to implement it (before RC). |
|
@liangmiQwQ Following the XDG directory convention will allow Vite+ to be more widely accepted, and many mature developer tools follow this convention, such as Claude Code and uv astral-sh/uv#8420 |
ad9d7dd to
5cc3b6c
Compare
|
@naokihaba If you have time, could you help take a look at whether this change has any impact on Nushell's functionality? |
|
@fengmk2 Sure, I'll check it. |
|
I will take over this PR and fix the issues discovered during the testing process. |
Capture the resolution design, grandfathering of existing ~/.vite-plus installs, installer alignment, and follow-ups for VP_HOME cleanup and migrate-on-upgrade.
Replace get_vp_home / the monolithic home module with VpDirs: category roots (bin, data, cache, config, state) come from an ordered resolution chain in dirs/resolution.rs (legacy Exist-gated grandfathering, VP_* and XDG Set overrides, then platform defaults), and first-level data subdirs (current, js_runtime, package_manager, packages, bins) are pure joins. Files and deeper trees stay with their features. Wire EnvConfig so test_guard/for_test_with_home isolate install roots without process-env leaks. Groundwork for voidzero-dev#827.
Replace every get_vp_home()-era dependency with EnvConfig::get().dirs.
Business directory helpers stay in their owning modules but are
reimplemented on the category roots, so no call site constructs install
paths ad hoc.
- env setup scripts re-export the captured layout overrides
(EnvConfig::env_exports) instead of hardcoding a VP_HOME line;
vp-use.cmd does the same on Windows
- env doctor reports all five category roots; bin/config are hard
errors when missing, data/cache/state are reported as lazily created
- implode deletes the deduped {data, cache, config, state} roots and
removes only vp-owned files from the bin directory, which may be a
shared ~/.local/bin
- upgrade-check cache moves to the state root; shim resolve cache
lives under the cache root
Fresh installs write the split platform roots; an existing ~/.vite-plus or --install-dir stays on the monolithic tree. Directory env vars stay owned by EnvConfig: vp-setup reads only EnvConfig::get().dirs, and a local vp binary reports those roots via VP_DUMP_DIRS so install.sh / install.ps1 / install-global-cli do not re-implement the chain. - Unix shims target <DATA>/current/bin/vp after env setup --refresh - Windows trampoline finds ../current or ../data/current without reading VP_HOME / VP_*_DIR - Standalone-install CI pins VP_HOME for published pre-split CLIs and adds a local-build job for fresh split + grandfather
CI still looks for $HOME/.vite-plus/bin/vp after bootstrap-cli:ci. Create that directory before EnvConfig resolves so the existing-install probe wins, without setting VP_HOME. Also clone rolldown/vite before the local-build layout job so `cargo build -p vp_global_cli` can load the workspace.
The test fetches the Node version index after scanning the cache. Without an EnvConfig::with_vars_async pin it can observe a concurrent test's VP_NODE_DIST_MIRROR=https://mirror.example/node and fail DNS lookup.
Implode only removes shims that resolve to this install's vp. Generated env files persist resolved VP_*_DIR. Legacy Windows wrappers pin VP_HOME to the data root. Upgrade-check cache moves to <CACHE>. Org-tarball cache goes through EnvConfig.
Independent VP_BIN_DIR / VP_DATA_DIR installs cannot be found by sibling-path probing. Write <name>.shim next to each trampoline and resolve vp.exe from that pointer. Align install.ps1 with Windows known folders so installer roots match EnvConfig.
install.sh now links <BIN>/vp to the absolute <DATA>/current/bin/vp path. The grandfather layout job still asserted the old relative ../current/bin/vp target.
Avoid the PowerShell 5 $HOME collision in install.ps1. Require a matching <name>.shim before implode deletes Windows exes. Pin VP_HOME from the trampoline for single-root bin/data layouts. Write JetBrains node paths from getVpDirs(). Persist VP_HOME in dir_envs only when it is an absolute path.
Pre-split binaries resolve every path from VP_HOME (default ~/.vite-plus), so installing one into split roots produced a broken PATH vp next to a half-built legacy tree while still exiting 0. install.sh, install.ps1, and vp-setup now probe the downloaded payload with VP_DUMP_DIRS=1 before the layout is final: a split-aware binary reports its category roots, which the installer adopts verbatim, and a binary that cannot answer falls back to the monolithic root (VP_HOME or ~/.vite-plus) with a notice. This also keeps fresh installs of latest working until the first split-aware release ships. vp-setup returns the effective directories for its success summary and removes the abandoned split data root that its managed node/pnpm downloads land in when this run created it. New test-install-sh-old-version (Linux, macOS) and test-install-ps1-old-version (Windows) CI jobs install 0.2.9 with no VP_HOME and assert the legacy layout and working PATH-resolved commands. The mechanism is documented in rfcs/directory-layout.md and the VP_VERSION entry of docs/guide/installer-env-vars.md.
with_vars_home_yields_split_layout pinned only HOME/USERPROFILE, so a shell that exports VP_HOME (vp's own env script does) or XDG_* resolved those overrides instead of the platform tail and failed. Clear every layout override var in the test.
960d13f to
9c48fa6
Compare
A pre-split local vite-plus can create ~/.vite-plus at any time on a machine whose global install is split (caches, config, managed runtimes). Because the grandfather source outranks VP_*_DIR, that stray tree captured the split install: vp upgrade and reinstalls silently moved to the monolithic root while the split PATH entries kept serving the old binary. Gate grandfathering on the current link every global install activates, checked without following it so a dangling link from a crashed upgrade still counts. install.sh and install.ps1 mirror the same gate; vp-setup inherits it through EnvConfig. The test-install-sh-layout CI job seeds a real install (version dir plus current link) for the reuse step and gains a stray-tree step: split install, stray ~/.vite-plus, then resolution and a reinstall must stay on the split roots.
…setup Apply cleanup findings from a four-angle review of the take-over commits: - Define the monolithic category mapping once per language: resolution::single_root_dirs backs VpDirs::legacy_single_root, set_monolithic_layout backs install.sh, New-MonolithicLayout backs install.ps1 (each replaced three hand-synced copies). - vp-setup skips the probe when the resolved dirs already equal the monolithic root, and otherwise adopts the payload's reported data/bin/config verbatim like the scripts do, so the written layout cannot drift from the binary's own resolution. - Share the VP_DUMP_DIRS category keys between printer and parser, and a LAYOUT_OVERRIDE_VARS list between the test env-clearing sites. - install.ps1's grandfather gate checks the current link without following it (Get-Item -Force), matching the sh and Rust gates for dangling links; unblock only the probed binary instead of a second blanket pass. - Hoist constant installer env to job level in the layout CI job and reference a job-level VP_VERSION in the pre-split jobs.
|
@codex review |
Apply an STE-style pass to the prose the take-over commits added. Split long multi-clause sentences in the RFC, use active voice, and replace the arrow chains with plain sentences. Use one term per concept: monolithic root/layout in prose (code symbols keep legacy names), pre-split for the release age. Reword the fallback notice from "predates the split directory layout; installing to <root>" to "does not support the split directory layout; the install goes to <root>" in install.sh, install.ps1, and vp-setup, with the CI greps and the RFC quote synced. Verified against a live 0.2.9 install.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 032b86f314
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| HashMap::from([ | ||
| (env_vars::VP_BIN_DIR, dirs.bin.as_path().to_string_lossy().into_owned()), | ||
| (env_vars::VP_DATA_DIR, dirs.data.as_path().to_string_lossy().into_owned()), | ||
| (env_vars::VP_CACHE_DIR, dirs.cache.as_path().to_string_lossy().into_owned()), | ||
| ]) |
There was a problem hiding this comment.
Preserve config and state roots in persisted environments
When vp env setup runs with a one-shot XDG_CONFIG_HOME or XDG_STATE_HOME and its generated environment is later sourced without that variable, this map pins only bin, data, and cache. The child then resolves config and state from platform defaults, so config.json, session state, and later setup output diverge from the installation that generated the environment. Fresh evidence beyond the earlier XDG comment is that the final implementation materializes only these three VP_* variables while claiming to reproduce identical roots; persist the effective config/state inputs as well.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Assessed as correct but an edge: it triggers only when a one-shot XDG_CONFIG_HOME/XDG_STATE_HOME is set during vp env setup and absent later. Persisting config/state needs new VP_CONFIG_DIR/VP_STATE_DIR variables in the resolution chain (only bin/data/cache have override vars today), and the current XDG-as-session-policy behavior is documented as deliberate. Leaving this open as a design decision.
| } | ||
|
|
||
| fn child_dir_pins(bin_dir: &std::path::Path, data: &std::path::Path) -> ChildDirPins { | ||
| if bin_dir == data.join("bin").as_path() { |
There was a problem hiding this comment.
Record split layout instead of inferring it from path shape
When a supported split Windows install sets, for example, VP_DATA_DIR=D:\vp and VP_BIN_DIR=D:\vp\bin without setting VP_HOME, this equality misclassifies it as a single-root install. Every trampoline consequently injects VP_HOME=D:\vp, relocating cache, config, and state under the data root instead of using the split roots selected by VpDirs; the sidecar needs to record layout provenance or enough roots rather than deriving the mode from this valid directory relationship.
AGENTS.md reference: AGENTS.md:L47-L47
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Assessed as correct but narrow: it needs VP_DATA_DIR/VP_BIN_DIR values that exactly mimic the monolithic shape (bin == data\bin). The clean fix records layout provenance in the .shim sidecar, which is a format change to the sidecar contract. Leaving this open as a follow-up decision rather than folding it into the pre-split compatibility work.
Grandfathering now requires the current link, so the bare ~/.vite-plus directory the CI seeding created no longer selects the monolithic root. The bootstrap then installed vp under ~/.local/bin while ci.yml and the snapshot jobs look for ~/.vite-plus/bin, which failed eight jobs. Pin VP_HOME for the CI bootstrap instead; the pin reaches the VP_DUMP_DIRS probe and the install script through process.env.
On a split install, vp upgrade installed a pre-split payload into the split data root and activated it; the old binary then moved config, runtimes, and later upgrades to ~/.vite-plus while the split PATH entries kept serving the old version. Gate the target version before the download: below 0.3.0 is rejected with a message that names the minimum version, preview builds (0.0.0-commit.<sha>) are allowed, and monolithic installs (VP_HOME pin or grandfathered ~/.vite-plus) accept every release, so the existing CI upgrade tests keep their targets. The upgrade path uses a version gate, not the payload probe: the running binary is split-aware by definition, so the boundary is a fixed release number. Move probe_payload_dirs into vp_setup with unit tests; vp-setup keeps using it, the upgrade path does not.
Close #827
Summary
Centralize on-disk paths in
vp_shared::VpDirsand default fresh installs to the split XDG/platform layout. Existing default installs under~/.vite-plusremain on the legacy monolithic root for now (grandfathered via existence-gated resolution). Automatic layout migration is out of scope here and tracked as follow-up work.Resolution (high level)
Ordered sources (per category): deprecated
VP_HOME(custom full-root pin) → existing~/.vite-plus(exist-gated legacy mapping) →VP_BIN_DIR/VP_DATA_DIR/VP_CACHE_DIR→XDG_*→ platform defaults.~/.vite-plus)~/.local/bin<root>/bin~/.local/share/vite-plus<root>~/.cache/vite-plus<root>/cache~/.config/vite-plus<root>~/.local/state/vite-plus<root>Windows split uses
%LOCALAPPDATA%\vite-plus\{bin,data,cache,state}and%APPDATA%\vite-plusfor config. RelativeVP_*/XDG_*values are treated as unset.Commits (review order)
docs(rfc): document split directory layout via VpDirsrfcs/directory-layout.mddescribing the resolution chain, grandfathering, installer alignment, and follow-ups (VP_HOMEcleanup, migrate-on-upgrade).env-command.md) are left unchanged.feat(shared): introduce VpDirs with strategy-gated path resolutionget_vp_home/home.rswithVpDirs+dirs/resolution.rsstrategy chain.current,js_runtime, …) stay as joins on the owning feature.EnvConfigso tests isolate layout viawith_vars/scopedwithout process-env leaks.AGENTS.mdpointer to the central path API.refactor(cli): route on-disk paths through VpDirsjs_runtime, package manager, implode, env setup/doctor, and related helpers ontoEnvConfig::get().dirs.dir_envs); implode deletes deduped{data, cache, config, state}and neverrm -rfs a shared bin dir.feat(install): default installers to the split XDG layoutinstall.sh/install.ps1/vp-setup/ trampoline /install-global-cliinstall into the rootsEnvConfigalready resolved. Fresh install → split; existing~/.vite-plusor--install-dir→ monolithic root.EnvConfig.vp-setupdoes not readVP_HOME/VP_*_DIR/XDG_*itself. A localvpbinary reports<DATA>/<BIN>/<CONFIG>viaVP_DUMP_DIRS=1so scripts andinstall-global-clido not re-implement the chain.vpshims target<DATA>/current/bin/vp(not a version dir) afterenv setup --refresh. The Windows trampoline finds../currentor../data/currentfrom disk layout only.VP_HOME-pinned; a local-build job covers fresh split + grandfather.User impact
~/.vite-plusVP_HOME~/.local/binneeds to be on PATHFollow-up
VP_HOMEusage in the repo (especially PTY snapshot tests); preferVP_*_DIR/ XDGvp upgrade, migrate default~/.vite-pluscontents into splitVP_*_DIR/XDG_*/ platform dirs and remove the legacy rootTest plan
cargo check/ unit tests fordirsresolution and layout helpersVP_HOME/ disable shared seed where fakes are used)install.sh/install.ps1)install-global-clivp-setup/ trampoline / Unix shim unit tests for fresh split vs grandfathertest-standalone-install(releasedVP_HOME-pinned + local-build split/grandfather jobs)~/.vite-plusstill works without moving